Fix coarse-to-fine cell maps for hierarchies built on an adaptively refined mesh - #5321
Merged
Merged
Conversation
coarse_to_fine_cells assumed that the cells kept by the non-overlapped plex were the leading cells of the overlapped one. That holds for a plex distributed by DMPlexDistributeOverlap, which appends its halo, but not for one built by DMPlexTransform, which numbers cells by refinement case and so interleaves owned with halo cells. Mask the absent cells and scatter each surviving entry to its non-overlapped number, rather than dropping them and slicing. Rewrite the docstring and comments of coarse_to_fine_cells to name the three cell numberings it moves between. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
#5215 did not touch the uniform refinement code (which is assuming that halo cells are always ordered last). An adaptively refined mesh interleaves owned and halo cells (nowhere else in firedrake we assume any ordering of the owned/halo cells). Therefore breaking the assumption in the uniform refinement routine is the right fix. |
pbrubeck
commented
Aug 4, 2026
pbrubeck
enabled auto-merge (squash)
August 4, 2026 22:25
leo-collins
pushed a commit
that referenced
this pull request
Aug 6, 2026
) coarse_to_fine_cells assumed that the cells kept by the non-overlapped plex were the leading cells of the overlapped one. That holds for a plex distributed by DMPlexDistributeOverlap, which appends its halo, but not for one built by DMPlexTransform, which numbers cells by refinement case and so interleaves owned with halo cells. Mask the absent cells and scatter each surviving entry to its non-overlapped number, rather than dropping them and slicing. Rewrite the docstring and comments of coarse_to_fine_cells to name the three cell numberings it moves between. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bugfix for a regression introduced by #5215.
Building a
MeshHierarchyon a mesh returned byrefine_marked_elementsfailed in parallel, asserting0 <= ccell < ncoarseinmgimpl.coarse_to_fine_cells.Root cause.
coarse_to_fine_cellsshould reindex the coarse mesh's plex-to-Firedrake cell map from overlapped to non-overlapped cell numbering, which it does by sending every local cell through the overlapped local-to-global map and back through the non-overlapped one. It assumed that the cells surviving that round trip were the leading cells of the overlapped plex: it discarded the rest (MapMode.DROP) and sliced. Meshes distributed byDMPlexDistributeOverlapappend their halo cells, so the assumption held for every mesh a hierarchy could be built on. #5215 added adaptive refinement, whose plex comes fromDMPlexTransform; that numbers cells by refinement case and interleaves owned with halo cells, so the slice was the wrong permutation, cells were attributed to the wrong parent, and the assertion fired.Fix.
coarse_to_fine_cellsnow masks the absent cells (MapMode.MASK) and scatters each surviving entry to the non-overlapped number the round trip gives it. For a mesh whose owned cells do lead its plex, this yields the same permutation as before.The docstring and comments of
coarse_to_fine_cellsare rewritten: numpydoc, and naming the three cell numberings the function moves between (Firedrake, overlapped plex, non-overlapped plex), none of which the ordering assumption was previously stated against. A test covers a uniform hierarchy built on an adaptively refined mesh.AI was used for this change (Claude Code).